Skip to content

Enable module state_dict compression, simplify compression logic #302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Apr 28, 2025
Merged

Enable module state_dict compression, simplify compression logic #302

merged 7 commits into from
Apr 28, 2025
+169 −165

Conversation

kylesayrs
Copy link
Contributor

@kylesayrs kylesayrs commented Apr 22, 2025 •
edited
Loading

Purpose

  • Enable ModelCompressor.compress to compress individual modules, as is required by Reduce memory requirements
  • Reduce logic required for mapping modules to schemes
  • Reduce logic required for skipping zero_point saving

Changes

  • map_modules_to_quant_args -> map_module_to_scheme
    • Narrow the return type from a str, qargs, or tuple of qargs to just the scheme applied to that module
    • Reduce logic which parses this unnarrowed return type
  • SimplifyModelCompressor.compress
    • Condense zero_point handling into _skip_zp
    • Handle compressing individual modules

@kylesayrs
use map_module_to_scheme, _should_save_zp
97bda13 
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs
remove unused import
6060bbe 
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs kylesayrs changed the title [Cleanup] map_module_to_scheme, _should_save_zp Enable module state_dict compression, simplify compression logic Apr 22, 2025
@kylesayrs kylesayrs requested review from dsikka and rahul-tuli April 22, 2025 21:04
@kylesayrs
remove unused imports
9e82ddb 
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs
rename to _skip_zp
f324af7 
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs kylesayrs mentioned this pull request Apr 22, 2025
Merged
rahul-tuli
rahul-tuli previously approved these changes Apr 23, 2025
View reviewed changes
Copy link
Member

@rahul-tuli rahul-tuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! great job!

@kylesayrs
type hint nit
83fcffd 
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs
safetensors function signature is weird
ddca6a5 
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
dsikka
dsikka requested changes Apr 23, 2025
View reviewed changes
Copy link
Collaborator

@dsikka dsikka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to run all our e2e tests before this can land.

brian-dellabetta
brian-dellabetta approved these changes Apr 23, 2025
View reviewed changes
@kylesayrs
Copy link
Contributor Author

kylesayrs commented Apr 23, 2025 •
edited
Loading

✅ https://github.com/neuralmagic/llm-compressor-testing/actions/runs/14624966862

@kylesayrs kylesayrs requested a review from dsikka April 24, 2025 14:26
dsikka
dsikka approved these changes Apr 24, 2025
View reviewed changes
Copy link
Collaborator

@dsikka dsikka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, we should wait to merge until after release

@kylesayrs kylesayrs enabled auto-merge (squash) April 28, 2025 15:15
@kylesayrs kylesayrs merged commit 4438d08 into main Apr 28, 2025
1 check passed
@kylesayrs kylesayrs deleted the kylesayrs/map_module_to_scheme branch April 28, 2025 15:16
kylesayrs added a commit that referenced this pull request Apr 28, 2025
@kylesayrs
Revert "Enable module state_dict compression, simplify compression lo…
2846b6a 
…gic (#302)"

This reverts commit 4438d08.
@kylesayrs kylesayrs mentioned this pull request Apr 28, 2025
Merged
dsikka pushed a commit that referenced this pull request Apr 28, 2025
@kylesayrs
Revert "Enable module state_dict compression, simplify compression lo…
1068c84 
…gic (#302)" (#306)

This reverts commit 4438d08.
@kylesayrs kylesayrs mentioned this pull request Apr 28, 2025
Merged
@brian-dellabetta brian-dellabetta restored the kylesayrs/map_module_to_scheme branch April 28, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brian-dellabetta brian-dellabetta brian-dellabetta approved these changes

@dsikka dsikka dsikka approved these changes

@rahul-tuli rahul-tuli rahul-tuli left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kylesayrs @brian-dellabetta @dsikka @rahul-tuli